Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add some doc aliases to Vec #80432

Closed
wants to merge 3 commits into from
Closed

Add some doc aliases to Vec #80432

wants to merge 3 commits into from

Conversation

KamilaBorowska
Copy link
Contributor

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 28, 2020
@KamilaBorowska KamilaBorowska changed the title Add "Vector" as doc alias to Vec Add some doc aliases to Vec Dec 28, 2020
@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Dec 28, 2020 •
edited
Loading

Actually, may as well add ArrayList and List too. ArrayList comes from Java, and List comes from Python. I think putting an alias for List is fine, most of the time if you want a list, Vec is probably the best choice.

pickfire
pickfire reviewed Dec 28, 2020
View reviewed changes
library/alloc/src/vec.rs Outdated
Comment on lines 299 to 301
#[doc(alias = "ArrayList")]
#[doc(alias = "List")]
#[doc(alias = "Vector")]
Copy link
Contributor

@pickfire pickfire Dec 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#[doc(alias = "array_list")]
#[doc(alias = "list")]
#[doc(alias = "vector")]

We should keep it snake_case like rust style, python also called it list, not List (although that is the typing type).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot this is a struct, can just ignore it. Sorry.

@bors
Copy link
Contributor

bors commented Dec 31, 2020

☔ The latest upstream changes (presumably #80530) made this pull request unmergeable. Please resolve the merge conflicts.

@KamilaBorowska KamilaBorowska force-pushed the add-vector-as-doc-alias branch from cb3fe39 to 8b6c100 Compare December 31, 2020 10:32
@KamilaBorowska
Copy link
Contributor Author

Weird, there apparently was merge conflict, but using git pull --rebase origin master automatically dealt with it.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2021
@camelid
Copy link
Member

camelid commented Feb 26, 2021 •
edited
Loading

cc #81989 — probably one or the other of these will have to be changed, they can't both land as-is

@JohnCSimon
Copy link
Member

@xfix - Ping from triage, can you please resolve the merge conflict? Thank you.
@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2021
@camelid
Copy link
Member

camelid commented Mar 17, 2021

@xfix Please rebase instead of merging; rust-lang/rust follows a "no-merge policy". Thank you!

@KamilaBorowska KamilaBorowska force-pushed the add-vector-as-doc-alias branch from 3bab103 to ec8b6b4 Compare March 18, 2021 06:49
@JohnTitor JohnTitor added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 18, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@Dylan-DPC-zz
Copy link

r? @m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned cramertj Apr 6, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Apr 21, 2021

Thanks for your PR and sorry for the late reply. This has been sitting still without review because it's not quite clear what exactly we want to use doc(alias) for.

See #81989 (comment).

While I do personally thik that doc(alias = "vector") would be a good one to add (since that's how we pronounce 'Vec' most of the time), we'd first like to reach some agreement on what kind of aliases we want and don't want before continuing merging PRs like this, as mentioned in the linked comment:

[..] we want some kind of clear guideline/policy that we can all agree on before accepting more aliases. Feel free to open an issue on https://github.com/rust-lang/libs-team or ping us on Zulip if you (anyone) want to work on this.

So, I'm closing this PR for now. Once we have a policy in place and this PR fits within it, feel free to re-open it.

@m-ou-se m-ou-se closed this Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pickfire pickfire pickfire left review comments

Assignees

@m-ou-se m-ou-se

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.